LfAction refactoring and area interchange target action support #1172
Conversation
obrix
changed the title
obrix
changed the title
|
Note for the reviewer : There was no specific rule about crashing or outputing a warning when an action cannot be applied so I kept more or less the same behavior for each action. Some of the warning / exception raising have been moved from construction to action application though. In my opinion I would prefer to raise warning and eventually have something in the output API to query which action has been applied rather that raising an exception for potentially a single wrong action and losing all the analysis. |
|
To sum up a bit, there is 4 cases of action setup that causes an exception raise : • For TapChangerAction, if the PiModel is a SimpleModel (no tap changer on the transformer) -> throw UnsupportedOperationException Those exception are the same as before the refactoring. All other potential issue are only outputting a warning, for example a branch not found in the network for a TerminalsConnectionAction. I also added a global warning for failing to apply an action in LfActionUtils, which might lead to a lot of warning when doing a security analysis on multiple component. So not sure we should keep that. |
| acParameters.getNetworkParameters().setAreaInterchangeControl(false); | ||
|
|
||
| LfAction lfAreaTargetAction = LfActionUtils.createLfAction(targetAction, network, acParameters.getNetworkParameters().isBreakers(), lfNetwork); | ||
| assertFalse(lfAreaTargetAction.apply(lfNetwork, null, acParameters.getNetworkParameters())); |
There was a problem hiding this comment.
The behavior should change very soon, after #1156 's merge.
Because of the ContingencyLoadFlowParameters extension, AIC can be performed on some post-contingency cases even if networkParameters.isAreaInterchangeControl() is false. So Area-s will now always be created, and this assertion will fail
There was a problem hiding this comment.
Thanks I will update if 1156 is merged before this one.
src/main/java/com/powsybl/openloadflow/network/action/LfActionUtils.java
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/network/action/LfActionUtils.java
Show resolved
Hide resolved
jeandemanged
changed the title
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
…alysis. Signed-off-by: Bertrand Rix <[email protected]>
…failure of applying an action. Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
obrix
force-pushed
the
area_interchange_target_action_support
branch
from
6267896 to
3ec6d46
Compare
|
|
|
||
| @Override | ||
| public boolean apply(LfNetwork network, LfContingency contingency, LfNetworkParameters networkParameters) { | ||
| LfHvdc lfHvdc = network.getHvdcById(action.getHvdcId()); |
There was a problem hiding this comment.
I'm adding here a question about the architecture change (not only specific on HVDC).
This change is not purely about architecture.
It changes also at what time static verifications (independant from the contingence) are done.
Before:
- Where done at action creation time (once per SA run). In case of issue the action was silently ignored (now they are reproted and it is better)
With this PR: - Verifications are performed for each contingence that needs them (innefficient - or can the action validity depend on the contingence ?)
- But the result in case of inconsitency is displayed in context of the contingence via a report (good)
Can't we get the best of both world by checking the action validity at creation time, and generating the report at apply time ?
There was a problem hiding this comment.
Good questions !
I can think of some scenario where the action application validity could depend on the contingency :
- With the work currently being done on load flow parameters specific to some contingencies : might impact the succesful application of an action.
- With some work planned on Artelys side to take into account the equivalent of "auto" action / SPS from a RAO that might be dependent on a type of contingency and then impact differently a same action done in different N-1 scenario. Maybe that's a bit early to care about that though but thats a possibility.
- More simply an action trying to impact an equipment that was indirectly impacted by a specific contingency
I think most checks / actions application are very fast (a lookup + eventually a boolean check + a call to one or more setter). I am not too worried about that at the moment.
Some of the checks like null branch, null area could probably be removed from the apply method because they are redondant with the checkActions function though.
Maye be some checks can definitely be moved at construction time like you suggested for the generator. I'll look at it again.
| } | ||
| } else { | ||
| throw new UnsupportedOperationException("Generator action on " + action.getGeneratorId() + " : configuration not supported yet."); | ||
| } |
There was a problem hiding this comment.
By checking this at apply time and not at creation time, you loose the fail fast property that existed before.
Before:
If there was an issue it was raised at launch time. User could see it immediately.
With his PR:
For a long computation, the issue may happen hours after launch time
=> User looses time
=> The user may not see the issue if lost in a large log.
There was a problem hiding this comment.
Indeed this can probably be moved at construction time. I will do it.
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
No issue
What kind of change does this PR introduce?
Add the support of AreaInterchangeTarget action and change the implementation of LfAction.
LfAction existing design was confusing as everything was handled in the unique LfAction object.
A proper class hierarchy has been setup so as the make feature extension and maintenance easier.
What is the current behavior?
All actions implemented in the LfAction object. Created through the create method and applied through the global update connectivity method (for switch and terminal connection actions) and apply method.
Each instance of LfAction was capable of describing all kind of actions through all the available attributes but a single instance was created per iidm action.
What is the new behavior (if this is a feature change)?
All action creation and application processes are implemented in dedicated class, with common code in AbstractLfAction for all actions and additional AbstractLfBranchAction for switch and terminal connection action (they both impact topology).
Applying the action can be done through a call to the apply method or through a call to LfActionUtils.applyListOfActions. This last method, in a similar way that was done before the refactoring, apply a list of action by first applying the actions modifying topology and then the rest of the actions.
Does this PR introduce a breaking change or deprecate an API?
If yes, please check if the following requirements are fulfilled
What changes might users need to make in their application due to this PR? (migration steps)
Other information: